(12/26) 141 - Drag'n Drop BufferTabs

- **assigned_to**: nobody --> arobert

I'm trying it out now. I can drag, but I can't seem to drop. Is it supposed to allow me to move a buffer from one EditPane to another? Or just reorder the tabs in a single editpane? Because if I am sorting my buffers by name, then there is no point in drag and drop unless it can go between editpanes and views.

- **assigned_to**: arobert --> nobody

In fact I don't understand. As far as I remember it was already possible to drag buffers to reorder them (if the buffers are not sorted by jEdit) isn't it ?
This patch add a "glass panel", that's a nice feature, but does it add something else ?

@kpouer The BufferTabs plugin shows through tabs all the files that you have opened. But it doesn't allow you to visually rearrange them. This patch does that. GlassPane is there in order to draw over the JTabbedPane to show the tab being dragged and to indicate where the tab will be dropped to.

@ezust You should be able to drop, the icon just says you can't. I fixed that in the updated patch (along with some fixes to minor PMD warnings about unnecessary nested if statements). If you do a split screen, you can only drop the tab in the same screen. You can't drag a tab from one screen and drop it into a spot on a second screen. You can, however, reorder the tabs differently among the screens.

I tried the updated patch. I am still unable to drop into another split pane.
I am able to re-order the tabs only if buffersets are not sorted.
And while that was possible before, the visual feedback that is given now during a drag is a slight improvement.

Thanks for pointing out that drop doesn't work when buffersets are sorted. I will try to fix this as soon as possible.

Can you please explain how drag and drop should work between split panes? Are you talking about keeping the panes synchronized so that when I move a tab in one pane it gets moved in the other pane? I kind of like being able to rearrange the tabs independent of the pane that they are in.

If your bufferset scope is not EditPane, then drag and drop between editpanes should be forbidden.
And if your buffersets are sorted, drag and drop on the same editpane should also be forbidden.

A very nice feature would be that if you drag outside a view from jEdit, it creates a new plain view with the buffer it it.

Another nice feature would be: if the bufferset scope is not global, then dnd between views should work too.

Drag-n-Drop-BufferTabs.patch (17.2Kio)

Updated Patch.
Added Feature: When user drags tab to undroppable region, create a new plain view with buffer in it.

Todd, you made many unnecessary changes which caused reindentation of large parts of code. These changes are cosmetic. Cosmetic changes are here joined with significant functionality changes. This makes it hard to understand the patch. Hard to analyze it.

I'm also sceptic about copy and paste of large parts of foreign code. Here it is done without much care, resulting in putting awt imports in 2 separate places. There's so much code inside a constructor, that is actually not constructing code, but definition of classes. We usually don't do this.

I won't be the one to commit it, at least not in current shape. And please don't extend this patch further. Next changes should be introduced partially, on the top of this one.

jarek, I don't understand the last 2 sentences of your previous comment.
For me, the patch seems readable, but it just doesn't work quite right yet.
My problem with the patch is that I still can't dnd between split panes, and if I try, it creates a new view when I drop instead of moving the tab to the other pane.

Instead of creating a new view when dropped in an "undroppable region", it should create a new view only when you drop OUTSIDE any existing view. Responding to drops in an undroppable region is very confusing to the user and should not be done in BufferTabs.

- **status**: open --> pending-rejected

Alan, the last 2 sentences. Todd submitted a patch having a given functionality in mind. The patch is quite complex. And I see comments: you should also do ..., it would be nice to have ... too. I think the patch should be first applied with the basic functionality, once the problems are fixed.

Extensions and wishes should come later. So it looks like you didn't apply the patch, because Todd didn't extend it in the direction you liked. Strange.

- **assigned_to**: nobody --> ezust

- **status**: pending-rejected --> open-rejected

@jarekczek

The changes I made outside of drag and drop were to fix PMD warnings about having nested if statements. That should have made the code cleaner. (I'm sorry, if I don't use the same indentation as you.)

If you had actually looked at the foreign code you would have seen that I didn't just copy and paste it. I actually took some time to understand what it does. But even if I had done like what you said I'd still have to figure out how to modify it to work with JEdit.

The classes you're talking about are only instantiated in the constructor. That's why I made them anonymous. I could have made separate classes for them, but that would have been unnecessary and made the code even more complex than what it already is.

I'm not sure what other problems there are (other than not implementing all of the requested functionality).

> Todd submitted a patch having a given functionality in mind.
There was no new functionality at all introduced by the first version of this patch. DND on the same editpane already works and does the same thing before or after this patch - it just doesn't show you the nice icon while you are dragging.

The second version of the patch does introduce new functionality and it's buggy - redefines what an "invalid drop area" means in drag and drop terminology, and confuses the user as to what to expect when the mouse button is released. Those two reasons are why I am rejecting it.

If the patch actually did something useful, I'd accept it, but currently it does nothing useful (yet).

When it does work, it should allow dnd for a single editpane only when buffersets are not sorted, and otherwise FORBID the drop.

It should also allow dnd from one editpane to another, and move the buffertabs between them, but only if the bufferset scope is editpane.

Matthieu's request is for drop outside a view, to create a new view. That would be nice too and it is not implemented in this patch yet.

And my request for dnd between views (when non-global bufferset scope is in use) would be a nice thing to have too.

- **status**: open-rejected --> open

I think the idea to see the destination of the tab being moved is worth introducing. Although personally I don't like the big icon which obscures background. I vote for accepting the original patch, if the reviewer accepts all the coding style.

Alan, that's the way we do things. Partially and with well separated pieces. Like that:
1. introduce drag and drop using java.awt.dnd
2. use glass pane to mark the insertion
3. support dragging to another pane
Todd joined 1,2. It's enough to merge 2 functionalities in one patch. I guess 3 is not possible without 1, so applying 1 seems necessary to achieve your goal.

Todd:
> I could have made separate classes for
>them, but that would have been unnecessary and made the code even more
>complex than what it already is.
Function bodies should be no more than one screen long. That's one of the rules of readability. There are necessary exceptions, but in this case, it's not at all necessary to make the constructor 5 screens long.
We may differ in the opinion on anonymous classes' readibility and complexity. But it's a fact, that they make code impossible to browse with sidekick.

>I'm sorry, if I don't use the same indentation as you.
Don't be. Just adjust to the rules that are in use in the file you edit.

- **assigned_to**: ezust --> jarekczek

I just added a tabSize=4:indentSize=4:noTabs=false: to the top of each source file in buffertabs in svn.
Jarek, if you want to accept a version of the first patch, that's ok with me. I'm sure you'll ensure it is cleaned up before you accept it.
My original feature request is #2108386, in case anyone is interested.

- **assigned_to**: jarekczek --> nobody